Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Honor the ${pcfiledir} entries in pkgconfig #977

Merged
merged 11 commits into from
Jan 24, 2025
Merged

fix: Honor the ${pcfiledir} entries in pkgconfig #977

merged 11 commits into from
Jan 24, 2025

Conversation

sergio-costas
Copy link
Contributor

@sergio-costas sergio-costas commented Jan 16, 2025 •
edited
Loading

The ${pcfiledir} tag allows to make .pc files relocatables, by being replaced with the path where the .pc file is stored. In these cases, the "prefix=" entry must be left as-is, to ensure that it continues to work as expected.

It is complemented by canonical/snapcraft#5157

Fix canonical/snapcraft#5158

  • Have you signed the CLA?
  • Have you added an entry to the changelog (docs/reference/changelog.rst)?

This was referenced Jan 16, 2025
Merged
Closed
@sergiusens sergiusens requested review from mr-cal and bepri January 18, 2025 00:20
bepri
bepri requested changes Jan 21, 2025
View reviewed changes
Copy link
Contributor

@bepri bepri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks great! Just two requests:

craft_parts/packages/normalize.py Show resolved Hide resolved
docs/reference/changelog.rst Outdated Show resolved Hide resolved
mr-cal
mr-cal reviewed Jan 21, 2025
View reviewed changes
Copy link
Contributor

@mr-cal mr-cal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, I don't have anything to add on top of @bepri's review, so please re-request me when you address his feedback. Thanks!

@sergio-costas sergio-costas force-pushed the honor-pcfiledir-in-pkgconfig-files branch from 79ca8cc to ddbd41c Compare January 22, 2025 09:15
@sergio-costas sergio-costas requested a review from bepri January 22, 2025 09:16
@sergio-costas
Copy link
Contributor Author

@mr-cal It seems that the sphinx spelling check dislikes pcfiledir. Do you know how can I avoid that error?

@bepri
Copy link
Contributor

bepri commented Jan 22, 2025 •
edited
Loading

@mr-cal It seems that the sphinx spelling check dislikes pcfiledir. Do you know how can I avoid that error?

There's a word whitelist file at docs/common/craft-parts/craft-parts.wordlist.txt that you can add "pcfiledir" to

@sergio-costas
Copy link
Contributor Author

Thanks, I'll fix it right now.

bepri
bepri approved these changes Jan 22, 2025
View reviewed changes
Copy link
Contributor

@bepri bepri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@bepri bepri requested a review from mr-cal January 22, 2025 14:00
@sergio-costas
Copy link
Contributor Author

@mr-cal Reviewed complete by @bepri

@sergio-costas sergio-costas force-pushed the honor-pcfiledir-in-pkgconfig-files branch from 116f8c0 to 546e27e Compare January 23, 2025 08:43
@sergio-costas
Copy link
Contributor Author

Rebased against main to fix a conflict.

mr-cal
mr-cal approved these changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@mr-cal mr-cal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

docs/reference/changelog.rst Show resolved Hide resolved
@sergio-costas
fix: Honor the ${pcfiledir} entries in pkgconfig
6b59cb7 
The ${pcfiledir} tag allows to make .pc files relocatables, by
being replaced with the path where the .pc file is stored. In
these cases, the "prefix=" entry must be left as-is, to ensure
that it continues to work as expected.

Fix #5158
@sergio-costas
Add changelog entry
24d62f0
@sergio-costas
Fix rst file
c6bf9a7
@sergio-costas
Another try
02e4b19
@sergio-costas
Should fully fix documentation
6c9358d
@sergio-costas
Fix changelog
6f741fb
@sergio-costas
Added extra comments
f63f15f 
These explains why ${pcfiledir} is checked.
@sergio-costas
Fix changelog
f78484b
@sergio-costas
Modify comment to avoid error in HTML
83fbc7f
@sergio-costas
Add dictionary exception for pcfiledir
3fb43ca
@sergio-costas
Add new entry in changelog.rst
19e74d6
@sergio-costas sergio-costas force-pushed the honor-pcfiledir-in-pkgconfig-files branch from 848207c to 19e74d6 Compare January 24, 2025 14:32
@mr-cal mr-cal merged commit b4a09d9 into canonical:main Jan 24, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mr-cal mr-cal mr-cal approved these changes

@bepri bepri bepri approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Snapcraft doesn't honor ${pcfiledir} in pkgconfig files
3 participants
@sergio-costas @bepri @mr-cal